First review at head f04a8835. I checked the branch out and exercised channel_history_content_for_user_turn directly rather than trusting the green badge. The core fix is correct and the scope is tight (one function, one file). All 18 checks pass. 🟢

🟢 The fix is correct — verified against a real [IMAGE:<path>] marker

The bug in #8151 is that a deferred image's filesystem reference was discarded from history. I confirmed the fix resolves it: feeding [IMAGE:/work/matrix_files/shot.png]\n\nwhat is this? through the patched function returns the marker intact ([IMAGE:/work/matrix_files/shot.png]\n\nwhat is this?), so a later turn can re-load the file. Operating on the original content and only .replace()-ing data: URIs is the right shape — is_loadable_image_reference already treats data: as the only inline-payload class, so the predicate matches the strip exactly.

🟢 Correctly scoped

Single function, no collateral churn, _cleaned discard is appropriate now that the cleaned output is unused. cargo clippy -p zeroclaw-channels is clean (no unused-variable warning).

🔴 The two regression tests do not exercise the fix

Both tests use the prose form [Image: sticker.webp attached], which parse_image_markers does not recognize as a loadable marker (it lacks the [IMAGE: prefix and a loadable ref). So image_refs.is_empty() is true and the function early-returns content unchanged — the new loop body never runs. channel_history_content_for_user_turn_strips_inline_image_payload and strip_historical_image_payloads_preserves_current_turn_payload would both still pass even if the entire fix were reverted. There is zero coverage for the actual #8151 scenario.

Required: add a test with a real [IMAGE:/abs/path.png] marker asserting the path survives in history, and a companion [IMAGE:data:image/png;base64,...] case asserting the inline payload is stripped. Those two are the behavior this PR exists to change; right now nothing pins them.

🔴 PR body claims the placeholder text changed, but it did not

The body states "Changes the placeholder text to indicate re-loading availability," yet the diff still returns the literal "[Image attachment processed by vision model]" for the empty-content branch. That is issue #8151's complaint #1 verbatim (the placeholder asserts vision processing even when the image was only acked and deferred). For an image-only turn with no caption, the misleading text is unchanged. Either update the placeholder as the body promises, or correct the body to not claim a change that isn't there.

🟢 Minor, non-blocking — wrapped data-URI strip can miss

format!("[IMAGE:{}]", ref_path) reconstructs from the collapsed ref (parse_image_markers runs collapse_wrapped_marker before pushing to image_refs). If a data: URI in the source was line-wrapped, the formatted needle won't match the wrapped original, so .replace() no-ops and the base64 stays in history — the bloat this branch is meant to prevent. Filesystem paths are unaffected (they're intentionally kept). Worth a follow-up, not a blocker.

Net: the fix is right and proven. Holding the verdict open for the test gap and the body/placeholder discrepancy — both are quick to close.

I checked this out locally (pull/8167/head, f04a883), built zeroclaw-channels, and traced the strip logic against parse_image_markers. The intent is right — deferred filesystem image refs should survive in history — but the implementation has a correctness hole that defeats its own stated goal, plus it conflicts with master and skips the template.

🔴 Blocking — the base64 strip silently misses line-wrapped markers

parse_image_markers (crates/zeroclaw-providers/src/multimodal.rs:203) pushes the collapsed marker text into refs: each candidate goes through collapse_wrapped_marker (line 181) before refs.push(candidate). So image_refs holds the normalized, un-wrapped form.

This PR then strips with:

result.replace(&format!("[IMAGE:{}]", ref_path), "")

which reconstructs the literal marker from the collapsed ref_path and matches it against the original, un-collapsed content. For any [IMAGE:data:...] marker that was line-wrapped in the source — and base64 data URIs are long, so they are the most likely thing to be wrapped — the collapsed string will not match the original wrapped text, the .replace() is a no-op, and the full base64 payload stays in history. That is exactly the case the function exists to prevent ("inline base64 too large for history"). The happy path (single-line data:) works; the case that actually bloats history does not.

The robust fix is to use the parser's structured output instead of string-rebuilding: walk image_refs, and for each non-data: ref re-emit its [IMAGE:ref] marker into the cleaned text (parse_image_markers already returns a correctly-cleaned body via cursor walking), dropping only data: refs. Don't reconstruct-and-replace against the raw input — that reintroduces the wrapping mismatch the parser already solved.

🔴 Blocking — conflicts with master

GitHub reports this CONFLICTING, and git merge-tree against current master confirms real conflict hunks. It needs a clean merge of master (not rebase) before it can land.

🔴 Blocking — PR body does not follow the required template

.github/pull_request_template.md is a merge gate. This body is ad-hoc and omits every required section. For a risk: high channel-history change these matter:

• Validation Evidence (required) — no commands, no tails. This is history-content behavior; add a unit test that feeds a wrapped [IMAGE:data:...] marker plus a [IMAGE:/path] marker and asserts the data URI is gone while the path survives. That test is also what would have caught the bug above.
• Security & Privacy Impact (required) — answer the four Yes/No. Note: base64 image payloads persisting in history is arguably a data-hygiene concern, which is the whole point.
• Compatibility (required) — structured Yes/No.
• Rollback — risk: high: fill fast-rollback path, toggles, observable symptoms.

🟢 The diagnosis is correct

For when this is reworked: the root cause you identified is real — the old code returned cleaned, which strips all [IMAGE:...] markers including re-loadable filesystem paths, so deferred attachments lost their reference and the bot denied seeing them on later turns (#8151). Keeping filesystem paths and URLs while dropping only inline base64 is the right contract. It just needs to be implemented on the parser's structured output so wrapping can't defeat it, with a test to prove it.